Skip to content

Conversation

@lukel97
Copy link
Contributor

@lukel97 lukel97 commented Mar 6, 2025

From what I understand, we only create VPReductionRecipes for in-loop reductions, and we don't currently support in-loop AnyOf reductions.

We only create VPReductionRecipes in the !PhiR->isInLoop() section of adjustRecipesForReductions, and this comment from the initial patch seems to confirm this https://reviews.llvm.org/D108136#anchor-inline-1038338, so I think we can remove this check in the condition logic.

I checked compiling SPEC 2017 with -prefer-inloop-predicates and the added assertion doesn't trigger.

From what I understand, we only create VPReductionRecipes for in-loop reductions, and we don't currently support in-loop AnyOf reductions.

We only create VPReductionRecipes in the !PhiR->isInLoop() section of adjustRecipesForReductions, and this comment from the initial patch seems to confirm this https://reviews.llvm.org/D108136#anchor-inline-1038338, so I think we can remove this check in the condition logic.

I checked compiling SPEC 2017 with -prefer-inloop-predicates and the added assertion doesn't trigger.
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2025

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Luke Lau (lukel97)

Changes

From what I understand, we only create VPReductionRecipes for in-loop reductions, and we don't currently support in-loop AnyOf reductions.

We only create VPReductionRecipes in the !PhiR->isInLoop() section of adjustRecipesForReductions, and this comment from the initial patch seems to confirm this https://reviews.llvm.org/D108136#anchor-inline-1038338, so I think we can remove this check in the condition logic.

I checked compiling SPEC 2017 with -prefer-inloop-predicates and the added assertion doesn't trigger.


Full diff: https://github.com/llvm/llvm-project/pull/130048.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp (+4-6)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index d154d54c37862..6ee98b717a910 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -2222,6 +2222,8 @@ void VPReductionRecipe::execute(VPTransformState &State) {
   assert(!State.Lane && "Reduction being replicated.");
   Value *PrevInChain = State.get(getChainOp(), /*IsScalar*/ true);
   RecurKind Kind = RdxDesc.getRecurrenceKind();
+  assert(!RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind) &&
+         "In-loop AnyOf reductions aren't currently supported");
   // Propagate the fast-math flags carried by the underlying instruction.
   IRBuilderBase::FastMathFlagGuard FMFGuard(State.Builder);
   State.Builder.setFastMathFlags(RdxDesc.getFastMathFlags());
@@ -2232,12 +2234,8 @@ void VPReductionRecipe::execute(VPTransformState &State) {
     VectorType *VecTy = dyn_cast<VectorType>(NewVecOp->getType());
     Type *ElementTy = VecTy ? VecTy->getElementType() : NewVecOp->getType();
 
-    Value *Start;
-    if (RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind))
-      Start = RdxDesc.getRecurrenceStartValue();
-    else
-      Start = llvm::getRecurrenceIdentity(Kind, ElementTy,
-                                          RdxDesc.getFastMathFlags());
+    Value *Start = llvm::getRecurrenceIdentity(Kind, ElementTy,
+                                               RdxDesc.getFastMathFlags());
     if (State.VF.isVector())
       Start = State.Builder.CreateVectorSplat(VecTy->getElementCount(), Start);
 

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@lukel97 lukel97 merged commit 6d89c04 into llvm:main Mar 6, 2025
11 checks passed
pawosm-arm pushed a commit to pawosm-arm/llvm-project that referenced this pull request Apr 10, 2025
…lvm#130048)

From what I understand, we only create VPReductionRecipes for in-loop
reductions, and we don't currently support in-loop AnyOf reductions.

We only create VPReductionRecipes in the !PhiR->isInLoop() section of
adjustRecipesForReductions, and this comment from the initial patch
seems to confirm this
https://reviews.llvm.org/D108136#anchor-inline-1038338, so I think we
can remove this check in the condition logic.

I checked compiling SPEC 2017 with -prefer-inloop-predicates and the
added assertion doesn't trigger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants